Skip to content

Detect wedged vGPU VFs from the guest and report to the health store - #435

Draft
yummybomb wants to merge 2 commits into
hypeship/vgpu-vf-quarantinefrom
hypeship/vgpu-wedge-quarantine
Draft

Detect wedged vGPU VFs from the guest and report to the health store#435
yummybomb wants to merge 2 commits into
hypeship/vgpu-vf-quarantinefrom
hypeship/vgpu-wedge-quarantine

Conversation

@yummybomb

@yummybomb yummybomb commented Aug 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Top half of the wedged-VF work, stacked on #462 (the VF health store, placement exclusion, admission, and /resources fields). This PR adds the detection path that feeds that store.

  • The guest agent watches /dev/kmsg for kernel-facility NVRM: ... RmInitAdapter failed! records, probes driver init at boot with nvidia-smi -L when the image has it, and exposes the current GPU initialization state over GetGPUInitStatus via vsock.
  • A host sentinel controller polls vendor-VFIO instances with up to 64 concurrent checks and reports failures and successes into the VF health store from Quarantine unhealthy vGPU VFs via a persisted health store #462.
  • Only instances with a live VMM (control socket present) are polled; a stopped or standby instance whose failed release left its claim in metadata is skipped, since QEMU vsock dials by guest CID alone and a stale CID could be reused by an unrelated instance.
  • Reports are keyed on GPUClaimedAt from Quarantine unhealthy vGPU VFs via a persisted health store #462, the identity a claim receives when it is persisted.

Rebuilt on the claim-first allocator

This branch was rebuilt from the new #462 head after #321 merged as the claim-first rewrite. The guest agent, guest RPC, sentinel controller, wiring, metrics, and docs are the previously reviewed code, squashed into one commit. What changed in the port:

  • GPUAssignedAt is gone from main; the sentinel keys reports on GPUClaimedAt, which Quarantine unhealthy vGPU VFs via a persisted health store #462 sets in the same metadata save as the claim.
  • The GPURetainedForCleanup skip is gone with the retention-record design. The existing control-socket check already excludes instances without a VMM.
  • The "allocate the vGPU immediately before metadata persistence" change to create.go is dropped. Claim-first already persists the claim before the VF is touched.
  • The reconcile liveness change is re-hooked into main's two hypervisorMayBeAlive call sites; reconcile and the create/start cleanup guard share one wrapper that logs the resolution error and increments the liveness counter when it fails closed.
  • The retention-record tests that the old branch adjusted no longer exist and were dropped with them.

Safety and failure handling

  • GPU initialization success is terminal in the guest agent, so a delayed failure watcher cannot replace a confirmed success.
  • The host trusts only the guest-agent RPC; workload output on the shared serial console cannot influence the tally.
  • A slow nvidia-smi attempt is killed after 30 seconds. The guest agent waits for that process to be reaped before retrying, so an attempt stuck in uninterruptible I/O cannot accumulate concurrent probes; the independent kmsg watcher still reports the underlying init failure.
  • Unreachable guests do not serialize a host-wide scan; checks run with a fixed concurrency limit.
  • Repeated polls and controller restarts do not double-count an assignment. The sentinel revalidates the VF assignment before reporting, and the health store deduplicates reports by assignment.
  • Reconciliation preserves claims when hypervisor liveness is uncertain and records that condition in logs and metrics.

Observability

  • hypeman_instances_vgpu_sentinel_init_failures_total
  • hypeman_instances_vgpu_sentinel_quarantines_total
  • hypeman_instances_vgpu_sentinel_checks_total by result (ok, failed, unknown, rpc_error, unsupported_agent, or list_error)
  • hypeman_instances_vgpu_quarantined_vfs
  • hypeman_instances_vgpu_vf_health_store_unavailable
  • hypeman_instances_vgpu_liveness_uncertain_total

lib/devices/GPU.md gains the detection and sentinel documentation.

Out of scope

An operator force-cycle endpoint. Recovery remains the documented manual DCGM quiesce, SR-IOV cycle, state edit, restart, and verification flow.

Testing

Passed locally:

go vet ./lib/instances ./lib/devices ./lib/guest ./lib/system/guest_agent ./lib/providers ./cmd/api
go test -race ./lib/devices ./lib/guest ./lib/system/guest_agent ./lib/resources ./lib/providers
go test -race ./lib/instances -run 'VGPU|Sentinel|Reconcile|Metric|HypervisorMayBeAlive|ResolveLiveHypervisor|Select|Quarantin|StoredVGPU|Cleanup|ReleaseStored|Snapshot.*GPU|Fork.*GPU'

cmd/api/wire_gen.go was regenerated with wire and matches. lib/guest/guest.pb.go and guest_grpc.pb.go are carried over from the previous head; guest.proto has not changed on main since, and the header records the same protoc and protoc-gen-go versions as the committed files.

The full lib/instances suite was not run here; tests that boot real VM images need a host this environment does not provide.

The underlying wedge signal and manual recovery sequence were previously validated on L40S hardware. A live end-to-end run of the guest watcher and host controller is still required before merge, including two failed assignments and the success path.

@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from d312338 to 7d54fb9 Compare August 20, 2026 19:12
@yummybomb
yummybomb marked this pull request as ready for review August 20, 2026 19:33
Comment thread lib/devices/vf_health.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 7d54fb9 to 9a90223 Compare August 20, 2026 20:17
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 9a90223 to 0ff16d1 Compare August 20, 2026 21:57
Comment thread lib/devices/vf_health.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 0ff16d1 to 3db5546 Compare August 20, 2026 22:04
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 3db5546 to b35501a Compare August 21, 2026 15:13
Comment thread lib/instances/vgpu_sentinel.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from b3bf925 to 55a0d6f Compare August 21, 2026 20:44
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 55a0d6f to ee1b160 Compare August 21, 2026 20:47
Comment thread lib/devices/vendor_vfio_linux.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch 2 times, most recently from a5dc229 to fed19c4 Compare August 24, 2026 19:03
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch 2 times, most recently from 7bd4f52 to 67b2b24 Compare August 24, 2026 19:34
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 67b2b24 to 0662ad9 Compare August 24, 2026 20:08
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch 2 times, most recently from f3bf0cc to ff54c20 Compare August 25, 2026 14:25
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from ff54c20 to 37bc190 Compare August 25, 2026 14:28
Comment thread lib/system/guest_agent/gpu_watch.go Outdated

@yummybomb yummybomb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

one blocking test-quality issue. the sentinel/store boundaries, fail-closed placement behavior, assignment revalidation, and persistence rollback paths otherwise look coherent in context. targeted race tests and go vet pass locally; the current linux CI also exposes the changed-test failure below.

require.NoError(t, os.WriteFile(nvidiaSMI, []byte(script), 0o755))
t.Setenv("COUNT_PATH", countPath)

probeGPUInitUntil(&gpuInitReporter{}, nvidiaSMI, time.Now().Add(time.Second), 10*time.Millisecond, 0)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker: this 10ms deadline is shorter than normal shell startup under race/loaded CI, so the nominally successful second attempt can time out too. The current Linux job failed here with actual: 3, and go test -race ... -count=100 reproduces it locally. Please make the attempt runner/clock injectable and use a deterministic fake, or otherwise give the success attempt independent scheduling margin; asserting exactly two real process launches with this deadline is flaky.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed in 88f3c3d by injecting the probe attempt into the retry loop and testing the timeout/success sequence directly. go test -race ./lib/system/guest_agent -run TestProbeGPUInitRetriesAfterAttemptTimeout -count=100, the package race suite, and go vet pass.

@yummybomb yummybomb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

two code-quality findings. the store/placement boundaries, persistence rollback, assignment revalidation, and tests otherwise look coherent in repo context. the new functions peak at cyclomatic complexity 16 (loadLocked / selectLeastLoadedVF); those paths remain cohesive and well-covered, so I don't see mechanical decomposition as a merge requirement.

Comment thread lib/instances/vgpu_sentinel.go Outdated
func (c *VGPUSentinelController) scanTarget(ctx context.Context, target vgpuSentinelTarget) {
tail := c.tails[target.instanceID]
if tail == nil || tail.vfAddress != target.vfAddress || tail.assignedAt != target.assignedAt {
tail = &vgpuSentinelTail{vfAddress: target.vfAddress, assignedAt: target.assignedAt}

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tail offset is process-local, but a successful report removes the only persistent dedup record. After FAILED -> OK, restarting hypeman starts at offset 0, records the historical failure again, increments init_failures_total, then clears it again on the historical OK. With vf_quarantine_threshold: 1, replay also emits a fresh quarantine/rescind and increments both counters on every restart; if that replayed clear fails, a healthy VF stays quarantined. Please retain a bounded last-resolved assignment/watermark per VF (or coalesce a replay before mutating state) and add a restart regression test.

Comment thread lib/instances/vgpu_sentinel.go Outdated
}

reader := bufio.NewReaderSize(f, vgpuSentinelMaxLineBytes)
for {

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: please thread ctx into scanSentinelLog and check cancellation between reads (and target scans). On the first pass after controller/process start every assignment begins at byte 0, so a host with many logs near the 50 MB rotation limit can keep Run inside this loop and hold the process-level errgroup.Wait well after shutdown was requested.

@yummybomb yummybomb left a comment

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

reviewed the change against hypeship/vendor-vfio-vgpu and traced the sentinel through guest output, host log scanning, durable health state, placement, and admission. the persistence rollback and assignment-identity handling are coherent, but the serial marker is not actually source-authenticated, and timed-out probes can overlap in the exact uninterruptible-ioctl case this code targets.

leaving the first finding as a blocker; github does not allow this account to request changes on its own PR.

Comment thread lib/instances/vgpu_sentinel.go Outdated

// Require a standalone guest-agent line so echoed marker text cannot count against a VF.
var (
vgpuSentinelFailedPattern = regexp.MustCompile(`^\d{4}/\d{2}/\d{2} \d{2}:\d{2}:\d{2} \[guest-agent\] (HYPEMAN-GPU-INIT-FAILED ts=\S+ nvrm="NVRM: [^"\r\n]*RmInitAdapter failed![^"\r\n]*")\r?\n?$`)

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocker: a standalone line does not establish that the guest agent wrote it. In exec mode the customer entrypoint's stdout/stderr is wired directly to the same serial console (lib/system/init/mode_exec.go:132-133), so an ordinary workload can print a complete line matching this regex without root or /dev/kmsg access. Repeating that across assignments can quarantine healthy VFs, and the runbook's statement that only a root guest can forge this is therefore incorrect. Please carry the report over a channel whose source the host can distinguish (for example a dedicated guest-agent RPC/transport), or otherwise authenticate the source; matching the log shape is not sufficient here.

case err := <-done:
return err
case <-timer.C:
_ = cmd.Process.Kill()

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this bounds how long the caller waits, but not the attempt's lifetime. If nvidia-smi is stuck in the uninterruptible ioctl described above, Kill does not make cmd.Wait return; the process and waiter goroutine remain live while probeGPUInitUntil launches another attempt after 15s. The 10-minute loop can therefore accumulate many concurrent stuck probes on the VF. Can we retain the in-flight attempt and avoid launching another until it is reaped (or stop retrying once an attempt cannot be killed), so there is at most one outstanding nvidia-smi?

yummybomb

This comment was marked as spam.

@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 88f3c3d to b901282 Compare August 27, 2026 17:25
@yummybomb
yummybomb changed the base branch from hypeship/vendor-vfio-vgpu to hypeship/vgpu-vf-quarantine August 27, 2026 17:27
@yummybomb yummybomb changed the title Detect and quarantine wedged vGPU VFs Detect wedged vGPU VFs from the guest and report to the health store Aug 27, 2026
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from b901282 to 1fd824a Compare August 27, 2026 17:29
Comment thread lib/instances/vgpu_sentinel.go Outdated
Comment thread lib/instances/vgpu_sentinel.go Outdated
Comment thread lib/system/guest_agent/gpu_watch.go Outdated
Comment thread lib/instances/vgpu_sentinel.go
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from a805d59 to aa66cd3 Compare August 28, 2026 14:44
Comment thread lib/devices/vf_health.go

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Cursor Bugbot has reviewed your changes using default effort and found 1 potential issue.

Fix All in Cursor

❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.

Reviewed by Cursor Bugbot for commit b23a654. Configure here.

Comment thread lib/instances/vgpu_sentinel.go
@yummybomb
yummybomb requested a review from sjmiller609 August 28, 2026 18:28
@yummybomb
yummybomb marked this pull request as draft August 31, 2026 14:51
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from b49bc73 to a53ca5c Compare August 31, 2026 16:00
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from a53ca5c to 86fb40e Compare August 31, 2026 20:53
The guest agent watches /dev/kmsg for kernel-facility NVRM RmInitAdapter
failures, probes driver init at boot with nvidia-smi -L when present,
and exposes the current GPU init state over GetGPUInitStatus via vsock.
A host sentinel controller polls every vendor VFIO instance with a live
VMM (control socket present) at bounded concurrency and reports failures
and successes into the VF health store, which quarantines a VF once
failures accumulate from enough distinct assignments. Reports are keyed
on GPUClaimedAt, the identity a claim receives when it is persisted, so
rescans and controller restarts cannot double-count an assignment.

Reconcile now fails closed on an ambiguous hypervisor ownership check
instead of treating it as a plain boolean: the claim is preserved, a
warning names the instance and device, and
hypeman_instances_vgpu_reconcile_liveness_uncertain_total counts it.

The sentinel repairs a failed health store load or persist once per poll
so reports do not retry it inline.
@yummybomb
yummybomb force-pushed the hypeship/vgpu-wedge-quarantine branch from 86fb40e to 322c599 Compare September 2, 2026 06:00
Build the sentinel's vsock dialer from the metadata already loaded for the
poll target instead of GetInstance, which derived full instance state and
could persist exit info or boot markers on every 5s tick.

Route the create/start cleanup guard through the same liveness wrapper as
reconcile so an uncertain hypervisor check logs and counts once; the
counter is renamed hypeman_instances_vgpu_liveness_uncertain_total. Move
the quarantined-VF and health-store gauges next to the other instance
metrics, and drop the nil check on a controller wire cannot leave nil.

Share one /dev/kmsg record reader between the GPU init watcher and the
clock keeper, log each failed nvidia-smi probe attempt with its output,
and report the actual retry window when the probe gives up.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants